Skip to content

Fix ICE when trying to convert ConstKind::Error to usize #113712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Fix ICE when trying to convert ConstKind::Error to usize #113712

wants to merge 1 commit into from
+66 −1

Conversation

syvb
Copy link
Contributor

@syvb syvb commented Jul 15, 2023

Fixes an ICE (#113021) caused by trying to convert a ConstKind::Error to a usize.

Instead of panicking in the call to eval_target_usize in prefix_slice_suffix, prefix_slice_suffix now bails and does nothing. If it sees a ConstKind::Error, then there has already been an error reported so not doing anything shouldn't be a problem.

Closes #113021.

r? @matthiaskrgr

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 15, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@BoxyUwU
Copy link
Member

BoxyUwU commented Jul 15, 2023

r? @BoxyUwU

please remove the first 5 commits

@rustbot rustbot assigned BoxyUwU and unassigned matthiaskrgr Jul 15, 2023
@syvb syvb force-pushed the fix-error-usize-ice branch from 226df21 to f0b6a13 Compare July 15, 2023 15:55
@syvb syvb force-pushed the fix-error-usize-ice branch from f0b6a13 to 2ee8eb2 Compare July 15, 2023 15:58
@cjgillot
Copy link
Contributor

Why is this code path even reached? If there is an error in the constant, the typeck results or THIR should be tainted, and MIR building should not happen. Any idea why this does not happen?

@syvb
Copy link
Contributor Author

syvb commented Jul 16, 2023

@cjgillot I bisected this (#113021 (comment)), and this ICE was probably caused by #99798 (which added ConstKind::Expr), but I'm not sure how exactly that PR caused the ICE.

@wesleywiser
Copy link
Member

I think @cjgillot is correct and MIR building shouldn't be running at all. The change is this PR is ok but I don't really understand why we should have to add it here (and if this we do need this here for some reason, there are probably other areas that will need similar treatment).

r? @cjgillot

@rustbot rustbot assigned cjgillot and unassigned BoxyUwU Sep 21, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Oct 12, 2023

r? @oli-obk
I was recently digging around in similar issues. We have lots of similar issues lurking just behind convenient query orderings.

You can use -Ztreat-err-as-bug to find the query backtrace of the original error and then check if there's a place in there where we can taint typeck or thir building to avoid mir building.

@rustbot rustbot assigned oli-obk and unassigned cjgillot Oct 12, 2023
@oli-obk oli-obk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 12, 2023
@bvanjoi bvanjoi mentioned this pull request Oct 22, 2023
Merged
@oli-obk
Copy link
Contributor

oli-obk commented Oct 23, 2023

closing in favor of #117046

@oli-obk oli-obk closed this Oct 23, 2023
@apiraino apiraino removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ICE: expected usize, got Const
9 participants
@syvb @rust-log-analyzer @BoxyUwU @cjgillot @wesleywiser @oli-obk @matthiaskrgr @apiraino @rustbot